Skip to content

Check arguments of attributes where no arguments are expected#155193

Open
JonathanBrouwer wants to merge 14 commits intorust-lang:mainfrom
JonathanBrouwer:args_used_check
Open

Check arguments of attributes where no arguments are expected#155193
JonathanBrouwer wants to merge 14 commits intorust-lang:mainfrom
JonathanBrouwer:args_used_check

Conversation

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

@JonathanBrouwer JonathanBrouwer commented Apr 12, 2026 •
edited by rustbot
Loading

View all comments

This PR does the following:

  • Add a debug assertion to rustc_attr_parsing, to ensure we never forget to check the arguments of a meta item again
  • Removes the unused #[derive(Clone)] from ArgParser as that would break this debug assertion
  • [BREAKING] Properly check that #[inline(always(...))] gets no arguments
  • [BREAKING] Properly check that #[instruction_set(arm::a32(...))] gets no arguments
  • [BREAKING] Properly check that #[macro_export(local_inner_macros(...))] gets no arguments.
    Fixes Invalid value accepted for #[macro_export(local_inner_macros)] #154977
  • [BREAKING] Properly check that #[used(compiler(...))] gets no arguments.
  • Properly check that #[optimize(size(...))] gets no arguments.
  • Properly check that #[coverage(on(...))] gets no arguments.
  • Properly check that #[rustc_dump_layout(debug(...))] gets no arguments.
  • Properly check that #[rustc_abi(debug(...))] gets no arguments.
  • Properly check that #![test_runner(arg(...))] gets no arguments.
  • Properly check that #[rustc_must_implement_one_of(arg(...))] gets no arguments.
  • Properly check that #[allow_internal_unstable(arg(...))] gets no arguments.
  • Properly check that #[unstable_feature_bound(arg(...))] gets no arguments.
  • Properly check that #[rustc_allow_const_fn_unstable(arg(...))] gets no arguments.
  • Properly check that #[rustc_if_this_changed(arg(...))] gets no arguments.
  • Properly check that #[rustc_then_this_would_need(arg(...))] gets no arguments.

r? @jdonszelmann

@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Apr 12, 2026

⚠️ #[rustc_allow_const_fn_unstable] needs careful audit to avoid accidentally exposing unstable
implementation details on stable.

cc @rust-lang/wg-const-eval

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann

@rustbot rustbot added A-attributes Area: Attributes (`#[…]`, `#![…]`) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 12, 2026
@JonathanBrouwer JonathanBrouwer mentioned this pull request Apr 12, 2026
Closed
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 12, 2026

@bors try

@rust-bors

This comment has been minimized.

Sign in to view
rust-bors Bot pushed a commit that referenced this pull request Apr 12, 2026
@bors
Auto merge of #155193 - JonathanBrouwer:args_used_check, r=<try>
d2ef8ec 
Check arguments of attributes where no arguments are expected
@JonathanBrouwer JonathanBrouwer added S-blocked Status: Blocked on something else such as an RFC or other implementation work. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. labels Apr 12, 2026
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@JonathanBrouwer JonathanBrouwer force-pushed the args_used_check branch 2 times, most recently from 3231b2a to 15b2265 Compare April 12, 2026 11:18
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 12, 2026

@bors try cancel
@bors try

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 12, 2026

Try build cancelled. Cancelled workflows:

rust-bors Bot pushed a commit that referenced this pull request Apr 12, 2026
@bors
Auto merge of #155193 - JonathanBrouwer:args_used_check, r=<try>
b98202a 
Check arguments of attributes where no arguments are expected
@rust-bors

This comment has been minimized.

Sign in to view
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Apr 12, 2026

☀️ Try build successful (CI)
Build commit: b98202a (b98202ad067d72e45cec8be3d5c15d86ef0fd086, parent: 540f43a224317d894a9a0710a8d67704f179a33c)

@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 12, 2026

@craterbot check

@craterbot
Copy link
Copy Markdown
Collaborator

craterbot commented Apr 12, 2026

👌 Experiment pr-155193 created and queued.
🤖 Automatically detected try build b98202a
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot craterbot added S-waiting-on-crater Status: Waiting on a crater run to be completed. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Apr 12, 2026
jakubadamw
jakubadamw reviewed Apr 12, 2026
View reviewed changes
Comment thread compiler/rustc_attr_parsing/src/interface.rs Outdated
@rust-bors

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@rust-bors

This comment has been minimized.

Sign in to view
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 29, 2026 •
edited
Loading

11 regressed and 2 fixed are spurious.
1 regression is real, spectest-0.1.2 uses the incorrect #[inline(always = true)] syntax. The bug that allowed this is not recent (i.e. not introduced by the new parsing infrastructure) and has existed for years

Given that there is breakage, let's ask for T-lang approval: @rust-lang/lang

Given that this is quite a niche crate, I propose to make a PR to this crate and accept the breakage.

@JonathanBrouwer JonathanBrouwer added T-lang Relevant to the language team I-lang-nominated Nominated for discussion during a lang team meeting. I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination labels Apr 29, 2026
@JonathanBrouwer JonathanBrouwer mentioned this pull request Apr 29, 2026
Open
@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented Apr 29, 2026

PR submitted to spectest: aalexandrov/spectest#1

@JonathanBrouwer JonathanBrouwer mentioned this pull request May 1, 2026
Merged
jdonszelmann
jdonszelmann reviewed May 1, 2026
View reviewed changes
Comment thread compiler/rustc_attr_parsing/src/parser.rs Outdated
jdonszelmann
jdonszelmann reviewed May 1, 2026
View reviewed changes
Comment thread compiler/rustc_attr_parsing/src/parser.rs Outdated
jdonszelmann
jdonszelmann requested changes May 1, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@jdonszelmann jdonszelmann left a comment •
edited by rustbot
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor changes

View changes since this review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 1, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 1, 2026

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@JonathanBrouwer
Copy link
Copy Markdown
Contributor Author

JonathanBrouwer commented May 1, 2026

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdonszelmann jdonszelmann jdonszelmann requested changes

+1 more reviewer

@jakubadamw jakubadamw jakubadamw left review comments

Reviewers whose approvals may not affect merge requirements

Requested changes must be addressed to merge this pull request.

Assignees

@jdonszelmann jdonszelmann

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination I-lang-nominated Nominated for discussion during a lang team meeting. needs-crater This change needs a crater run to check for possible breakage in the ecosystem. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Invalid value accepted for #[macro_export(local_inner_macros)]

6 participants

@JonathanBrouwer @rustbot @rust-log-analyzer @craterbot @jakubadamw @jdonszelmann